[CALCITE-6193] If a query has more than one subexpression that matche…#3624
[CALCITE-6193] If a query has more than one subexpression that matche…#3624JiajunBernoulli wants to merge 1 commit intoapache:mainfrom
Conversation
…s a materialized view, only the first is substituted
|
| + "LogicalCalc(expr#0..1=[{inputs}], deptno=[$t0])\n" | ||
| + " LogicalJoin(condition=[=($0, $1)], joinType=[inner])\n" | ||
| + " LogicalUnion(all=[true])\n" | ||
| + " LogicalCalc(expr#0..1=[{inputs}], deptno=[$t1])\n" |
There was a problem hiding this comment.
The 1879 and 1880 lines are still be replaced by mv's target descendant, it's parent may be shared with a wrong node, so I think this solution is not very is not very correct.
There was a problem hiding this comment.
If I don't commit, the RelNode is
LogicalCalc(expr#0..1=[{inputs}], deptno=[$t0])
LogicalJoin(condition=[=($0, $1)], joinType=[inner])
LogicalUnion(all=[true])
LogicalCalc(expr#0..1=[{inputs}], deptno=[$t1]) -- same as 1879
LogicalCalc(expr#0..4=[{inputs}], proj#0..1=[{exprs}]) -- same as 1880
LogicalTableScan(table=[[hr, emps]])
LogicalAggregate(group=[{1}])
EnumerableTableScan(table=[[hr, MV0]])
LogicalAggregate(group=[{0}])
LogicalCalc(expr#0..4=[{inputs}], deptno=[$t1])
LogicalTableScan(table=[[hr, emps]])
There are two LogicalCalc, do you mean this is Error?
There was a problem hiding this comment.
Yes, because these two LogicalCalc operators both are replaced by mv's target descendant, I think these temporary replacements should be undo after the real replacement has been done. This is the correct solution for this problem.
| final MutableRel childOrNext = | ||
| queryDescendant.getInputs().isEmpty() | ||
| ? next : queryDescendant.getInputs().get(0); | ||
| ? next : queryDescendant.getInputs().get(currentInputIndex); |
There was a problem hiding this comment.
I think getInputs.get(currentInputIndex) is just a walk-around to force traverse the missed nodes. Actually, preOrderTraverseNext method can traverse all the nodes of this plan tree, There is no need to add a new status to record the current node's position.
There was a problem hiding this comment.
It is a demo.
I think that it always get(0) is error.
We should find a better way to traverse the missing nodes.
There was a problem hiding this comment.
I think get(0) is correct, get(0) is just a starting point when traverse child-nodes, preOrderTraverseNext method can traverse the missing nodes.
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions. |

…s a materialized view, only the first is substituted